Start writing config pluck helpers #2514
Conversation
| @@ -54,7 +54,7 @@ init_site <- function(pkg = ".") { | |||
|
|
|||
| copy_assets <- function(pkg = ".") { | |||
| pkg <- as_pkgdown(pkg) | |||
| template <- purrr::pluck(pkg$meta, "template", .default = list()) | |||
| template <- config_pluck(pkg, "template") | |||
There was a problem hiding this comment.
Default to NULL for consistency; there shouldn't be any functional difference because NULL$foo and list()$foo both return NULL.
| @@ -110,7 +110,7 @@ test_that("data_navbar() can remove elements", { | |||
| right: ~ | |||
| ") | |||
|
|
|||
| expect_snapshot(data_navbar(pkg)) | |||
| expect_equal(data_navbar(pkg)$right, "") | |||
There was a problem hiding this comment.
Replaced with an equality test since we know what we expect here, and it's simple.
| package <- purrr::pluck(pkg, "meta", "template", "package") | ||
| if (!is.null(package)) { | ||
| package <- config_pluck_string(pkg, "template.package") | ||
| if (package != "") { |
There was a problem hiding this comment.
| if (package != "") { | |
| if (nzchar(package)) { |
There was a problem hiding this comment.
FWIW I find that style hard to read
| pkg, | ||
| "Can't find {cli::qty(missing)} component{?s} {.field {msg_flds}}.", | ||
| call = call | ||
| config_pluck_character <- function(pkg, |
There was a problem hiding this comment.
could this be called config_pluck_chr()
There was a problem hiding this comment.
and the string one, config_pluck_str()?
There was a problem hiding this comment.
I think these are more matches to rlang's check_ helpers than purrr's, because they're about selecting single values (that are sometimes vectors).
| error_call = caller_env()) { | ||
| if (is.character(x)) { | ||
| x | ||
| } else if (identical(x, list())) { | ||
| character() |
There was a problem hiding this comment.
I don't know 😄 But I think it must be some yaml special case where we want to handle an empty list.
R/config.R
Outdated
| error_path, | ||
| error_call = caller_env()) { | ||
| if (is_list(x)) { | ||
| if (!is.null(names) && !all(has_name(x, names))) { |
There was a problem hiding this comment.
based on my confusion in the tests before I read the error snapshots:
| if (!is.null(names) && !all(has_name(x, names))) { | |
| some_required_components_absent <- (!is.null(names) && !all(has_name(x, names))) | |
| if (some_required_components_absent) { |
names is maybe not specific enough, could it be "required" or so.
There was a problem hiding this comment.
has_required? names is a bit vague
There was a problem hiding this comment.
I tried has_names and I think it's pretty clear when read in context.
* Refactor `yaml_character()` to `config_pluck_character()` * Implement new `config_check_list()` * Implement `config_pluck_string()`
The next part of #1927.
This is not yet complete, but I think it's a good place for another review because I'm starting to set up the API for typed extraction of components from
_pkgdown.yml.@olivroy I'd love your thoughts too 😄